Refactor footprint changes #167
Open
+30
−33
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…into a single parameterized function extract_footprint(footprint_dict, footprint_key). Removed the print missing kwh/co2, footprint_dict since it was polluting the cell execution, and in all these cases footprint_dict was empty.
…he actual exception error reason, also while accounting the total number of trips where there were errors.
…red from prior labels
|
Testing Scenario: Dataset used: Execution of notebooks- generic_metrics, mode_specific_metrics - to lookup for inferred label changes Detailed Execution of notebooks using generate_plots.py script
Results:
Note: Everything looks good |
…book_inferred_data(,..,add_footprint). Use base_mode AIR to filter out the trip without air mode, instead of internal label
|
Fix for the following: Use base_mode to filter out the AIR related trips. Results:
Looks identical |
JGreenlee
reviewed
Oct 18, 2024
Comment on lines
+507
to
+509
| def extract_footprint(footprint_dict, footprint_key): | ||
| if footprint_key in footprint_dict.keys(): | ||
| return footprint_dict[footprint_key] |
| @@ -254,7 +257,7 @@ async def map_trip_data(expanded_trip_df, study_type, dynamic_labels): | |||
|
|
|||
| return expanded_trip_df | |||
|
|
|||
| async def load_viz_notebook_inferred_data(year, month, program, study_type, dynamic_labels, include_test_users=False): | |||
| async def load_viz_notebook_inferred_data(year, month, program, study_type, dynamic_labels, include_test_users=False, add_footprint=False): | |||
There was a problem hiding this comment.
For my understanding, is this cleanup from something that was meant to be added in a previous PR?
I saw that load_viz_notebook_data already had the add_footprint parameter, but load_viz_notebook_inferred_data was missing it
There was a problem hiding this comment.
No, this is not a cleanup which was meant to be added in the previous PR. We were using add_footprint parameter to compute footprint calculations in labeled trips in energy_calculations notebook. Earlier, we were not using add_footprint parameter for load_viz_notebook_inferred_data.
We need to pass add_footprint parameter as True to enable extraction of base_mode. Therefore, this change is required, such that we have base_mode as column added in the expanded_ct_inferred data frame. This enables us for filtering AIR as the base_mode, instead of using mode_confirm_w_other = 'air'.
| @@ -244,7 +246,7 @@ | |||
| " plot_and_text_stacked_bar_chart(expanded_ct, lambda df: (df.groupby(\"mode_confirm_w_other\").agg({distance_col: 'count'}).sort_values(by=distance_col, ascending=False)), \n", | |||
| " \"Labeled by user\\n\"+stacked_bar_quality_text_labeled, ax[0], text_results[0], colors_mode, debug_df, values_to_translations)\n", | |||
| " plot_and_text_stacked_bar_chart(expanded_ct_inferred, lambda df: (df.groupby(\"mode_confirm_w_other\").agg({distance_col: 'count'}).sort_values(by=distance_col, ascending=False)), \n", | |||
| " \"Labeled and Inferred by OpenPATH\\n\"+stacked_bar_quality_text_inferred, ax[1], text_results[1], colors_mode, debug_df_inferred, values_to_translations)\n", | |||
| " \"Inferred from prior labels\\n\"+stacked_bar_quality_text_inferred, ax[1], text_results[1], colors_mode, debug_df_inferred, values_to_translations)\n", | |||
There was a problem hiding this comment.
Good text change. More concise & clearer
Co-authored-by: Jack Greenlee <[email protected]>
Co-authored-by: Jack Greenlee <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Handle Future cleanups mentioned in #152 Changes:
Type fixed from relecant to relevant.
Refactor extract_co2 and extract_kwh to a single consolidated parameterized function extract_footprint()